-
Notifications
You must be signed in to change notification settings - Fork 2k
Added transaction support for saveAll in JdbcChatMemoryRepository #3158
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Sun Yuhan <[email protected]>
...c/main/java/org/springframework/ai/chat/memory/repository/jdbc/JdbcChatMemoryRepository.java
Outdated
Show resolved
Hide resolved
…pository. Signed-off-by: Sun Yuhan <[email protected]>
…pository. Signed-off-by: Sun Yuhan <[email protected]>
...c/main/java/org/springframework/ai/chat/memory/repository/jdbc/JdbcChatMemoryRepository.java
Outdated
Show resolved
Hide resolved
…pository. Signed-off-by: Sun Yuhan <[email protected]>
Signed-off-by: Sun Yuhan <[email protected]>
|
This approach won't enable participation in a surrounding outer scope transaction, I'll work on a solution. |
Indeed, but do we really need participating in outer scope transaction here? |
You're right; this approach indeed cannot participate in outer scope transaction. However, it seems that ChatMemoryRepository is inherently designed with ChatMemory in mind. In such a scenario, do we really have an "outer scope transaction" that needs to be involved? |
|
Who knows what people will do in the wild, it is a tiny change to the PR. |
|
Thanks @sunyuhan1998 merged in 0e1bb52 |
In the saveAll method of JdbcChatMemoryRepository, all historical messages of the current conversation are first deleted, and then the latest list of messages is saved. This operation should be transactional. This PR adds transaction support for it.